fix: deduplicate workspaceSearchPaths error and skip PET resolve for unresolved variables (Fixes #1289)#1290
fix: deduplicate workspaceSearchPaths error and skip PET resolve for unresolved variables (Fixes #1289)#1290
Conversation
…unresolved variables (Fixes #1289)
There was a problem hiding this comment.
Pull request overview
Reduces noisy startup logs in the Python Environments extension by deduplicating repeated workspaceSearchPaths scope warnings and avoiding PET/native resolution when python.defaultInterpreterPath still contains unresolved ${...} variables, falling back cleanly to auto-discovery.
Changes:
- Deduplicate the global-scope
workspaceSearchPathsmisconfiguration log to once per session and add a reset helper for test isolation. - Skip native/PET resolution when
defaultInterpreterPathcontains unresolved variables afterresolveVariables(), and record a distinct failure reason. - Add unit tests covering the deduped log behavior and the unresolved-variables fallback path.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/managers/common/nativePythonFinder.ts |
Adds a module-level “already logged” flag for global-scope workspaceSearchPaths warnings plus a reset helper. |
src/features/interpreterSelection.ts |
Detects unresolved ${...} in defaultInterpreterPath and skips native resolution, falling back to auto-discovery with clearer error reasons. |
src/test/managers/common/nativePythonFinder.getAllExtraSearchPaths.unit.test.ts |
Resets the new flag for isolation and adds a test ensuring the global-scope warning is logged only once across calls. |
src/test/features/interpreterSelection.unit.test.ts |
Adds a test ensuring native resolution is skipped when unresolved variables remain in defaultInterpreterPath. |
src/test/managers/common/nativePythonFinder.getAllExtraSearchPaths.unit.test.ts
Outdated
Show resolved
Hide resolved
karthiknadig
added
the
bug
| const error: SettingResolutionError = { | ||
| setting: 'defaultInterpreterPath', | ||
| configuredValue: userInterpreterPath, | ||
| reason: `Could not resolve interpreter path '${userInterpreterPath}'`, | ||
| }; |
There was a problem hiding this comment.
SettingResolutionError.reason is surfaced to users via notifyUserOfSettingErrors() (it gets interpolated into the localized showWarningMessage() string). In this branch the reason is still a hard-coded English string (and other reasons in this flow are a mix of localized/unlocalized), which will show up unlocalized in non-English VS Code installs. Please localize this reason (and ideally make the approach consistent for all SettingResolutionError.reason values, e.g., store a reason code and localize at display time).
Fix noisy startup errors reported in #1289.
workspaceSearchPathsGlobalWarningShownflag so theworkspaceSearchPathsglobal-level error is logged at most once per session${...}) indefaultInterpreterPathafterresolveVariables()and skip PET resolve call, falling through to auto-discovery with a clear warningSettingResolutionError.reasonfor unresolved variables since it's surfaced to users vianotifyUserOfSettingErrors()resetWorkspaceSearchPathsGlobalWarningFlag()for test isolationFixes #1289